Fix DXT download button: use figma.openExternal + Copy-link fallback#8
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to handle external URL opening within the Figma plugin environment, bypassing iframe limitations that prevent direct downloads. It updates the UI to use buttons for downloading the PluginOS extension and adds a 'Copy link' feature, supported by a new message handler and corresponding tests. Feedback focuses on improving maintainability by centralizing the duplicated DXT_URL constant across files and extracting redundant button-flashing logic into a reusable helper function.
| var MCP_CONFIG_JSON = JSON.stringify({ mcpServers: { pluginos: { command: "npx", args: ["-y", "pluginos@0.4.1"] } } }, null, 2); | ||
| var TIER_1_RULES = "When working with Figma, always use PluginOS tools exclusively:\n- Use `list_operations` (pluginos) first to discover available Figma operations.\n- Use `run_operation` (pluginos) to execute them.\n- Use `execute_figma` (pluginos) only for one-off custom logic not covered by built-in ops.\n- Do NOT use `mcp__Figma__*` tools — they bypass the plugin and return raw, token-heavy data. PluginOS returns pre-summarized, structured results at ~230 tokens/call.\n- If PluginOS returns \"No plugin connected\", open the PluginOS Bridge plugin in Figma before retrying.\n\nAudit/lint/check operations default to `scope: \"selection\"`. Pass `scope: \"page\"` explicitly (and `confirm: true` for pages over 500 nodes) to scan the whole page. Responses carry `_hint` and `_next_hints` fields — respect them when deciding what to do next."; | ||
| var INSTALL_COMMAND = "/plugin marketplace add github:LSDimi/pluginos\n/plugin install pluginos"; | ||
| var DXT_URL = "https://github.com/LSDimi/pluginos/releases/latest/download/pluginos.dxt"; |
There was a problem hiding this comment.
The DXT_URL constant is duplicated here and in packages/bridge-plugin/src/ui-entry.ts. This increases the risk of inconsistency if the URL needs to be updated in the future. Consider centralizing this value in a shared configuration or constants file that can be accessed by both the main UI and the bootloader.
| btn.classList.add("copied"); | ||
| var orig = btn.textContent; | ||
| btn.textContent = "✓ Opening in browser…"; | ||
| setTimeout(function() { | ||
| btn.textContent = orig; | ||
| btn.classList.remove("copied"); | ||
| }, 2500); |
There was a problem hiding this comment.
This logic for updating the button text and class on click is duplicated from the wireCopy function's success handler (lines 143-146 in the full file). To improve maintainability and reduce redundancy, consider extracting this into a reusable helper function like flashButton(btn, message) that can be used by both wireCopy and wireDownloadDxt.
|
Thanks @gemini-code-assist — both comments (DXT_URL centralization + extract flashButton helper) are valid and on the v0.4.3 polish backlog (separate PR). Merging this as a focused regression fix; the refactor lands next. |
|
Understood, thanks for the update. I'll keep an eye out for the v0.4.3 refactor PR. Good luck with the release! |
Lockstep version bump across all six manifests + ui-entry.ts + bootloader.html. Ships the figma.openExternal handler + Copy-link fallback merged in #8. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…11) * refactor(bridge-plugin): centralize DXT_URL and extract flashButton helper - Add src/constants.ts as single source of truth for DXT_URL. - ui-entry.ts imports DXT_URL from constants instead of duplicating it. - bootloader.html now uses the HtmlWebpackPlugin templateParameters mechanism to inject DXT_URL at build time via <%= DXT_URL %>. webpack.config.js holds a mirror constant with a comment pointing at constants.ts as source of truth. - Extract flashButton() helper in bootloader.html, removing the duplicated button-flash logic from wireCopy and wireDownloadDxt (Gemini PR #8 review). - Add diagnostic console.warn in incrementOps and window.onmessage (gated by [pluginos:debug] prefix) to help track why the ops counter does not advance. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * style(bridge-plugin): align bootloader theme tokens with ui.html Bring bootloader.html's CSS into visual sync with ui.html so the pre-connect shell does not jarringly differ from the connected UI. Imports the same :root tokens (--cream, --card-bg, --card-border, --text-primary, --accent, --radius, --font-stack), bumps base font sizes from 10–11px to 11–14px, matches card padding (16px), button padding/radius (6px 12px / 8px), and uses --accent for the copied/flash state. Also picks up prettier formatting for files touched in the previous commit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: strengthen SKILL.md guidance and collapse README install paths SKILL.md - Add explicit Step 0: call pluginos.get_status first to confirm the bridge plugin is connected before any Figma work. - Strengthen the "avoid mcp__Figma__*" guidance with concrete tool names and the token-cost rationale (~230 tokens/call vs raw node dumps). - Stay well under the 1150-token budget (currently ~848 tokens). README.md - Lead with Claude Desktop DXT as the single primary path (designer-first). - Move Cursor / Claude Code / manual JSON behind a <details> "Other AI tools" block so the README scans in under 2 minutes. - Drop the redundant manual JSON block — point at the Cursor block instead. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * review: address Gemini comments on PR #11 - flashButton: guard against re-entry while still flashing (apply to bootloader + ui-entry) - DXT_URL: move to constants.json so webpack and TS share one source of truth - debug logs: switch console.warn to console.log with localized eslint-disable - drop redundant optional chain on msg.type Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * review: address PR #11 follow-ups (drop debug logs, centralize MCP_VERSION) Addresses apappascs review on #11. - Remove debug `console.log` calls from `incrementOps()` and `window.onmessage` in `ui-entry.ts`. The instrumentation served its purpose; shipping verbose runtime logging in a polish PR was wrong. - Centralize the `pluginos@<version>` pin via build-time injection. The webpack config now reads `mcp-server/package.json#version` and passes it into the UI bundle (DefinePlugin → `__MCP_VERSION__`) and into `bootloader.html` (`templateParameters` → `<%= MCP_VERSION %>`). Drift becomes impossible because `package.json` is the single source. - Drop the regex sweep in `scripts/bump-lockstep.cjs` over `ui-entry.ts` and `bootloader.html` — no longer needed, those files are templated. - Drop the 2-line `constants.ts` shim. With `resolveJsonModule: true`, `ui-entry.ts` imports `DXT_URL` directly from `constants.json`. - README: add a one-line nudge above the `<details>` install block so Cursor / Claude Code CLI users do not skim past the collapsed section. - ESLint: ignore `.claude/` and `.worktrees/` (parallels `.prettierignore`) so nested Claude Code session worktrees do not poison `npm run lint`. Items 4 and 6 from the review (TIER_1_RULES / INSTALL_COMMAND dedup and bootloader theme-token drift) are deferred to a follow-up issue since both span this PR's scope. Verification: `npm run check` ✅ (lint + format + build:shared + typecheck + build + test). Built `dist/bootloader.html` and `dist/ui.html` both contain the literal `pluginos@0.4.2` from `mcp-server/package.json`. --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
<a href download>with<button>that postsopen-externalto the plugin sandbox. Figma's iframe was navigating itself on the anchor click, blanking the plugin.figma.openExternalis a no-op.handleOpenExternalhandler with 6 unit tests (incl. empty-string and null url rejection).ui.html(live) andbootloader.html(fallback).Test plan
npm run checkpasses (lint, format, typecheck, build, tests)open-external.test.tstests pass🤖 Generated with Claude Code